Set up type checking with pyrefly #26
Conversation
Internal Meta typechecker reported a couple of issues in recent PR comments forwardning feature [1]. Fix them with additional checks and local variables. [1] #25 Signed-off-by: Ihor Solodrai <[email protected]>
pyrefly [1] is a modern type checker, suggested as a successor of pyre-check [2]. Add pyrefly configuration to pyproject.toml Add pyrefly check run in main ci workflow. Following the recommendation from the docs, annotate existing errors detected by pyrefly [3]. We'll be removing them incrementally with new code changes. [1] https://github.com/facebook/pyrefly [2] https://github.com/facebook/pyre-check [3] https://pyrefly.org/en/docs/error-suppressions/#upgrading-pyrefly-and-other-changes-that-introduce-new-type-errors Signed-off-by: Ihor Solodrai <[email protected]>
danielocfb
left a comment
danielocfb
left a comment
There was a problem hiding this comment.
Looks good to me, thanks for taking this on.
| """ | ||
| tos = msg.get_all("To", []) | ||
| ccs = msg.get_all("Cc", []) | ||
| # pyrefly: ignore # implicit-import |
There was a problem hiding this comment.
Does the # implicit-import bit carry any semantics for the checker? I was under the impression the syntax would be
# pyrefly: ignore[implicit-import]?
There was a problem hiding this comment.
The annotations were generated by pyrefly command, so I guess that's the expected syntax.
| self._add_ci_files() | ||
|
|
||
| try: | ||
| # pyrefly: ignore # missing-attribute |
There was a problem hiding this comment.
Kind of shocking how many additional suppressions we need. Just curious, have you investigated why that is? Are the git bindings so bad, somehow?
There was a problem hiding this comment.
I haven't investigated in detail. My impression is that the type checker is quite defensive, while KPD codebase isn't rich enough with the type annotations.
This self.repo_local is set to None in constructor without an annotation, and then overwritten in fetch_repo_branch. I'm pretty sure pyrefly can't see that.
Similarly, it seems to not understand mocks, for example
kernel-patches-daemon/tests/test_branch_worker.py
Lines 352 to 354 in 20155c3
Hopefully, we'll clean it up bit by bit.
pyrefly [1] is a modern type checker, suggested as a successor of
pyre-check [2].
Add pyrefly configuration to pyproject.toml
Add pyrefly check run in main ci workflow.
Following the recommendation from the docs, annotate existing errors
detected by pyrefly [3]. We'll be removing them incrementally with new
code changes.
[1] https://github.com/facebook/pyrefly
[2] https://github.com/facebook/pyre-check
[3] https://pyrefly.org/en/docs/error-suppressions/#upgrading-pyrefly-and-other-changes-that-introduce-new-type-errors